Add settings tooltips and search to the web UI#387
Merged
Conversation
Help users quickly find settings and understand how each one works. Tooltips: a new delegated controller (static/v3/js/tooltips.js) drives an accessible (i) info tooltip that appears on hover, keyboard focus, and tap. A shared `help_tip` Jinja macro (partials/_macros.html) emits the trigger; the plugin config macro and the core settings partials now surface help text through it. Per the design, the always-visible field help paragraphs are folded into the tooltip to declutter the forms, and the hardware/display settings carry authored detail (default, range, recommendation). Search: a global header search box finds settings across every settings tab — even ones not yet opened — via a lazy client-side index built by scanning the same field markup (static/v3/js/settings-search.js). Selecting a result switches tabs, waits for the field to load, then scrolls to and flashes it. A per-tab filter box hides non-matching fields on the current tab. Plugin settings get tooltips for free by reusing each field's schema `description`; every settings field also gets a stable `setting-<tab>-<key>` anchor id for search navigation. Styling uses the existing --color-* theme vars so light/dark mode both work, and honors prefers-reduced-motion. Adds Flask render smoke tests that assert each settings partial ships tooltips, anchors, and a filter box. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014gZxznuxw8L92FUMBN3Nqz
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a v3 settings search and tooltip system: shared template macros and styling, a server-built searchable index, client-side search/filter behavior, header wiring, broader partial markup updates, and smoke tests covering anchors, tooltips, and the search index. ChangesSettings Search and Tooltip UI
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsSearchJS as settings-search.js
participant PagesV3 as pages_v3 /settings/search-index
participant DOM
User->>SettingsSearchJS: type query in header search
SettingsSearchJS->>PagesV3: fetch /v3/settings/search-index
PagesV3-->>SettingsSearchJS: fields JSON (cached or built)
SettingsSearchJS->>SettingsSearchJS: search(q) match terms
SettingsSearchJS->>DOM: renderResults grouped by tab
User->>SettingsSearchJS: select result
SettingsSearchJS->>DOM: setActiveTab, waitForElement, reveal section
SettingsSearchJS->>DOM: scroll to field and flash
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 273 |
| Duplication | 2 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Refactor the two new modules to clear the flagged patterns without any behavior change: - Build the search dropdown with DOM nodes + textContent instead of innerHTML string concatenation, removing the XSS sinks and the manual escapeHtml helper it needed. - Replace numeric index access (index[i], terms[j], opts[idx], currentResults[i]) with array iteration methods, NodeList.item(), and Array.prototype.at() to clear detect-object-injection. - Use === via a shared termsMatch() helper, optional-catch binding, and drop a useless initial assignment. Verified with ESLint (eslint:recommended + eslint-plugin-security) at zero findings and re-ran the headless-Chromium behavior test (tooltip, per-tab filter, global search navigation) — all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014gZxznuxw8L92FUMBN3Nqz
Extract isNodeHidden() and revealNode() helpers so revealAncestors drops below the cyclomatic-complexity threshold. No behavior change; verified with the headless-Chromium test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014gZxznuxw8L92FUMBN3Nqz
- Validate plugin ids against a strict allowlist (mirroring the server's _SAFE_PLUGIN_ID_RE) before they can appear in a fetch path, so the request URL is never built from unvalidated input (Codacy: user-controlled URL). - Document that the fetched HTML is parsed into an inert document (scripts never run, never inserted into the live DOM) purely to read field text for the search index. - Declare block-scoped locals with const instead of var where they were nested inside conditionals (Codacy: var not at function root). No behavior change; re-verified with ESLint and the headless-Chromium test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014gZxznuxw8L92FUMBN3Nqz
Move index building off the client so there is no client-side HTML fetching or DOM parsing (resolves Codacy's variable-fetch and DOMParser flags on the read-only, same-origin index build). - Add GET /v3/settings/search-index (pages_v3.py): renders the settings partials server-side and extracts each field's anchor id, key, label, tooltip, and section with a small stdlib HTMLParser, then caches the result keyed on the installed-plugin set. Parsing the rendered HTML keeps anchor ids identical to the live DOM, so the index cannot drift. - settings-search.js: buildIndex() now does a single fetch of the literal endpoint + .json(); removed the per-partial fetch loop, DOMParser, scanDoc, CORE_TABS, and the plugin-id allowlist. Search, keyboard nav, navigation, and the per-tab filter are unchanged. Net: fewer requests and no client-side HTML parsing. Verified with a new endpoint test in test_web_settings_ui.py (12 pass) and the headless-Chromium test (tooltip, filter, search navigate + flash all green). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014gZxznuxw8L92FUMBN3Nqz
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
web_interface/static/v3/js/tooltips.js (1)
160-161: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the
console.logfrom production.This debug log will appear in the browser console for every page load. Consider removing it or gating it behind a debug flag.
♻️ Proposed fix
- - console.log('[Tooltips] controller registered'); })();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web_interface/static/v3/js/tooltips.js` around lines 160 - 161, The debug console log in the tooltips initialization is leaking into production output; remove the `console.log('[Tooltips] controller registered')` from the `tooltips.js` module or gate it behind an explicit debug flag used by the controller setup. Keep the rest of the initialization IIFE unchanged and ensure the `controller registered` message is not emitted on normal page loads.web_interface/static/v3/app.css (1)
834-841: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd
max-heightandoverflow-y: autoto#settings-search-results.The dropdown has no height constraint. On smaller screens (e.g., a Raspberry Pi 7" touchscreen), a full result set can overflow the viewport with no way to scroll. The
.ssr-groupsticky positioning also needs a scroll container to function. Given the Raspberry Pi target, this is worth addressing.♻️ Proposed fix
`#settings-search-results` { background: var(--color-surface); border: 1px solid var(--color-border); border-radius: 0.5rem; box-shadow: var(--shadow-lg); z-index: 50; padding: 0.25rem; + max-height: min(60vh, 24rem); + overflow-y: auto; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web_interface/static/v3/app.css` around lines 834 - 841, The `#settings-search-results` dropdown needs a height constraint and scroll container support so it won’t overflow small screens and can enable sticky .ssr-group behavior. Update the `#settings-search-results` rule in app.css to add a max-height appropriate for the viewport and overflow-y: auto, keeping the existing visual styling intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/test_web_settings_ui.py`:
- Around line 120-142: The search-index test in test_search_index_endpoint
currently covers general, display, and wifi fields but misses the durations tab,
so it won’t catch regressions there. Update the assertions in
test_search_index_endpoint to include a representative durations anchor from the
settings search index, and verify it has the same required label/help/tab
metadata as the other checked fields.
In `@web_interface/blueprints/pages_v3.py`:
- Around line 232-237: The settings search index is missing the Durations
section because `core_tabs` in `settings_search_index()` does not include
`_load_durations_partial()`. Update the `core_tabs` list to add a Durations
entry using the existing tab pattern and the `_load_durations_partial` loader so
its `setting-*` fields are included in the indexed pages.
In `@web_interface/static/v3/js/settings-search.js`:
- Around line 52-71: The failure path in buildIndex is caching an empty array in
window._settingsIndex, which causes later calls to short-circuit and never retry
after a transient fetch error. Update buildIndex so the .catch handler does not
leave a truthy failed cache behind; instead clear the cached index state and
reset buildPromise so a later call can refetch the index. Keep the retry
behavior tied to buildIndex, window._settingsIndex, and buildPromise.
- Around line 311-316: `filterScope` currently falls back to `document`, which
can make `applyTabFilter` hide setting fields across unrelated tabs. Update
`filterScope` to return only a real nearest container (or nothing) instead of
`document`, and add a guard in `applyTabFilter` so it skips filtering when no
valid tab/content scope is found. Use the existing `filterScope` helper and its
caller to keep the search limited to the current tab.
- Around line 8-17: Update the top-of-file comment in settings-search.js so it
matches the current implementation: the global search no longer fetches each
tab’s partial or scans window.installedPlugins, and it now uses the server-side
/v3/settings/search-index endpoint. Keep the per-tab filter description if still
accurate, but rewrite the global search and plugin-loading notes to describe the
JSON index-based flow and avoid mentioning the removed client-side approach.
---
Nitpick comments:
In `@web_interface/static/v3/app.css`:
- Around line 834-841: The `#settings-search-results` dropdown needs a height
constraint and scroll container support so it won’t overflow small screens and
can enable sticky .ssr-group behavior. Update the `#settings-search-results` rule
in app.css to add a max-height appropriate for the viewport and overflow-y:
auto, keeping the existing visual styling intact.
In `@web_interface/static/v3/js/tooltips.js`:
- Around line 160-161: The debug console log in the tooltips initialization is
leaking into production output; remove the `console.log('[Tooltips] controller
registered')` from the `tooltips.js` module or gate it behind an explicit debug
flag used by the controller setup. Keep the rest of the initialization IIFE
unchanged and ensure the `controller registered` message is not emitted on
normal page loads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 33962156-5162-4bd2-9d00-d1c331939eab
📒 Files selected for processing (14)
test/test_web_settings_ui.pyweb_interface/blueprints/pages_v3.pyweb_interface/static/v3/app.cssweb_interface/static/v3/js/settings-search.jsweb_interface/static/v3/js/tooltips.jsweb_interface/templates/v3/base.htmlweb_interface/templates/v3/partials/_macros.htmlweb_interface/templates/v3/partials/backup_restore.htmlweb_interface/templates/v3/partials/display.htmlweb_interface/templates/v3/partials/durations.htmlweb_interface/templates/v3/partials/general.htmlweb_interface/templates/v3/partials/plugin_config.htmlweb_interface/templates/v3/partials/schedule.htmlweb_interface/templates/v3/partials/wifi.html
- pages_v3: include Durations tab in the search index so setting-durations-* fields are actually indexed - test_web_settings_ui: assert setting-durations-clock is present in the search-index endpoint response - settings-search.js: on index fetch failure, reset buildPromise instead of caching an empty (truthy) index so search can retry - settings-search.js: filterScope returns null (not document) when no tab container matches, and the caller guards, so the per-tab filter can't hide fields across unrelated tabs - settings-search.js: refresh the stale header comment to describe the server-side JSON index flow - app.css: cap #settings-search-results height with overflow-y so the dropdown scrolls instead of overflowing small screens Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014gZxznuxw8L92FUMBN3Nqz
Global search: - Close the results dropdown on input blur (guarded, with a short delay) so it reliably dismisses when focus leaves — previously it could linger because the only outside-close was a document click that Alpine/HTMX handlers can swallow. - Clear the query text after navigating to a result so refocusing the box doesn't re-open stale results. - Also dismiss on htmx:afterSwap (tab changes / navigation). Per-tab filter (now on plugin tabs too): - Render the shared settings_filter box in the plugin Configuration panel. It auto-wires: the delegated input handler and filterScope already target .plugin-config-tab. - Teach applyTabFilter to reveal matches inside collapsed nested sections (render_nested_section defaults them shut), hide nested-section wrappers with no matches, and restore the original collapsed layout when cleared (only re-collapsing sections the filter itself opened). - Count a visible nested-section as content for its parent heading so the heading isn't hidden while a subsection below still has matches. Adds a plugin-config render test (filter box + nested anchors + tooltips). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014gZxznuxw8L92FUMBN3Nqz
The dropdown could stay open after clicking away because the only outside-click close was a bubble-phase document listener. The v3 UI is one Alpine app() component full of HTMX/Alpine/widget click handlers; when a click lands inside an element that calls stopPropagation(), the event never bubbles to document and the close never runs. - Replace the bubble-phase document 'click' close with a capture-phase 'pointerdown' listener scoped to #settings-search-wrap. Capture runs before any bubbling stopPropagation can swallow the event, so it always fires; pointerdown also covers touch on the Pi screen. Clicking a result stays inside the wrap, so selection is unaffected. - Guard the debounced input handler so a delayed render can't re-open the box after focus has left (type-then-click-away race). Keeps the existing blur / Escape / htmx:afterSwap closes as secondary paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014gZxznuxw8L92FUMBN3Nqz
.hidden has no effect in this app: app.css is a hand-picked utility
subset (no Tailwind build step) and never defines .hidden { display:
none }. openResults()/closeResults() only toggled the class, so the
dropdown stayed rendered (display: block) even once closeResults()
ran - confirmed via computed style in a headless browser. Set
style.display directly, matching the fallback already used by
revealNode()/collapseNode() elsewhere in this file.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
Adds two features to the v3 web UI so users can quickly find settings and understand how each one works: an accessible (i) info tooltip on every setting, and a search — a global header box that finds settings across all tabs (even unopened ones) plus a per-tab filter. Help text that used to sit under each field is folded into the tooltip to declutter the forms, and hardware/display settings carry authored detail (default, range, recommendation).
Type of change
Related issues
Test plan
EMULATOR=true python3 run.py)scripts/dev_server.py)pytest)Details:
test/test_web_settings_ui.py, 11 cases) assert each settings partial ships tooltips (class="help-tip"), search anchors (id="setting-..."), and a per-tab filter box, and that authored rich detail is present — all pass.base.htmlshell through Flask (200s; header search box + script includes present).aria-describedby, hides on mouse-out; the per-tab filter hides/shows fields; the global search builds its index, shows a tab-grouped dropdown, and selecting a result switches tabs, scrolls to the field, and flashes it.node --checkpasses on both new JS modules; all templates compile.Documentation
README.mdif user-facing behavior changeddocs/if developer behavior changedHelp text shown in the new tooltips for the Display/hardware settings was sourced from the existing per-key reference in
README.md; no README change was needed.Plugin compatibility
Plugin settings get tooltips for free by reusing each field's existing schema
descriptionin theplugin_config.htmlmacro; no plugin or schema changes are required. The pluginenabledkey remains rendered as the header toggle (unchanged).Checklist
CONTRIBUTING.mdCONTRIBUTING.mdandCODE_OF_CONDUCT.mdNotes for reviewer
How it works
partials/_macros.html— shared Jinja macros:help_tip(text, label)emits an accessible<button class="help-tip" data-tooltip="…">;fg_id(tab, key)for anchor ids;settings_filter()for the per-tab filter box.static/v3/js/tooltips.js— one delegated controller for all.help-tiptriggers (hover, keyboard focus via:focus-visible, tap toggle, Esc/outside-click to close). Survives HTMX swaps with no re-init; positions a single#ledm-tooltippanel; text set viatextContent(XSS-safe) withwhite-space: pre-linefor authored\nline breaks.static/v3/js/settings-search.js— lazy, session-cached client index built by fetching each settings partial once and scanning the same.form-group[id^="setting-"]+<label>+data-tooltipmarkup, so it can't drift from what's rendered. Global dropdown (grouped by tab, full keyboard nav) +navigateToSetting(sets AlpineactiveTab,MutationObserver-waits for the field to appear — handling the two-stage plugin load — then scrolls + flashes). Per-tab filter is a delegatedinputhandler scoped to the active tab.app.cssuses the existing--color-*theme vars (light/dark automatic) and honorsprefers-reduced-motion.Decisions worth a look
🤖 Generated with Claude Code
https://claude.ai/code/session_014gZxznuxw8L92FUMBN3Nqz
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes